Skip to content

fix(cost-insights): bound rollup transaction lifecycle#4502

Merged
jeanduplessis merged 5 commits into
mainfrom
fix/cost-insight-rollup-lifecycle
Jul 13, 2026
Merged

fix(cost-insights): bound rollup transaction lifecycle#4502
jeanduplessis merged 5 commits into
mainfrom
fix/cost-insight-rollup-lifecycle

Conversation

@jeanduplessis

Copy link
Copy Markdown
Contributor

Summary

Prevent Cost Insights rollup transactions from outliving their tracked request lifecycle and creating prolonged database lock convoys.

Why this change is needed

Concurrent AI Gateway rollup writes serialize on one owner-hour unique-index entry. Detached post-commit transactions could finish SQL without reaching commit or rollback before serverless execution ended, retaining locks while queued writers consumed database connections.

How this is addressed

  • Attach organization usage and Cost Insights follow-up transactions to the existing tracked usage promise while preserving separation from the billing transaction.
  • Bound best-effort Cost Insights capture with transaction-local lock, statement, and idle-transaction timeouts without inline retries.
  • Keep optional failures from failing canonical usage recording and report only privacy-safe PostgreSQL error metadata.
  • Expand hot-key concurrency, lock-timeout, lifecycle, and canonical-repair regression coverage.

Human Verification

  • Awaited post-commit lifecycle helper smoke check passed.
  • Five-focus code review found no logic, data safety, security, resource, or type-contract issues.

Reviewer Notes

Human Reviewer Flags

  • Cost Insights capture remains post-commit and best-effort; timed-out deltas remain repairable from canonical microdollar_usage but do not create a durable dirty-owner marker.
  • Full batched derivation is intentionally deferred pending post-fix contention measurements.

Code Reviewer Agent

Code Reviewer Notes
  • Production AI Gateway usage already runs in an outer Next.js after() callback. Awaiting follow-up work extends that tracked promise while remaining valid for direct admin and script callers that lack request context.
  • AI Gateway Cost Insights capture uses 2s lock, 5s statement, and 10s idle-in-transaction limits. Other billing-critical rollup callers are unchanged.
  • Database-backed Jest tests were not run locally because Docker was unavailable; CI should execute added regression coverage.

Comment thread apps/web/src/lib/ai-gateway/processUsage.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.ts 618 Owner repair can double-count concurrent usage
apps/web/src/lib/ai-gateway/processUsage.ts 634 Failed repair still loses the owner-hour permanently

Fix these issues in Kilo Cloud

Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues
  • apps/web/src/lib/ai-gateway/processUsage.ts - 2 issues
Previous Review Summaries (3 snapshots, latest commit 1e4fcfc)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 1e4fcfc)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.ts 618 Owner repair can double-count concurrent usage
apps/web/src/lib/ai-gateway/processUsage.ts 634 Failed repair still loses the owner-hour permanently

Fix these issues in Kilo Cloud

Files Reviewed (2 files)
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues
  • apps/web/src/lib/ai-gateway/processUsage.ts - 2 issues

Previous review (commit 54d90c3)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.ts 564 Evaluation scheduling failures are recorded as capture gaps
apps/web/src/lib/ai-gateway/processUsage.ts 577 One owner's timeout degrades rollups for every owner

Fix these issues in Kilo Cloud

Files Reviewed (5 files)
  • apps/web/src/lib/ai-gateway/processUsage.test.ts - 0 issues
  • apps/web/src/lib/ai-gateway/processUsage.ts - 2 issues
  • apps/web/src/lib/ai-gateway/usage-post-commit-work.test.ts - 0 issues
  • apps/web/src/lib/ai-gateway/usage-post-commit-work.ts - 0 issues
  • apps/web/src/lib/cost-insights/rollup-maintenance.ts - 0 issues

Previous review (commit 015249c)

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/processUsage.ts 549 Timeout failures leave no durable repair signal

Fix these issues in Kilo Cloud

Files Reviewed (5 files)
  • apps/web/src/lib/ai-gateway/processUsage.ts - 1 issue
  • apps/web/src/lib/ai-gateway/usage-post-commit-work.test.ts - 0 issues
  • apps/web/src/lib/ai-gateway/usage-post-commit-work.ts - 0 issues
  • apps/web/src/lib/cost-insights/rollup-maintenance.integration.test.ts - 0 issues
  • packages/db/src/cost-insights-rollups.test.ts - 0 issues

Reviewed by gpt-5.6-sol · Input: 141.6K · Output: 19K · Cached: 1.1M

Review guidance: REVIEW.md from base branch main

Comment thread apps/web/src/lib/ai-gateway/processUsage.ts
Comment thread apps/web/src/lib/ai-gateway/processUsage.ts Outdated
Comment thread apps/web/src/lib/ai-gateway/processUsage.ts Outdated
Comment thread apps/web/src/lib/ai-gateway/processUsage.ts Outdated
@jeanduplessis jeanduplessis merged commit e22469d into main Jul 13, 2026
59 checks passed
@jeanduplessis jeanduplessis deleted the fix/cost-insight-rollup-lifecycle branch July 13, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants